Repository navigation
perf(napi): cache resolved namespace members - #27
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Namespace objects were Proxies whose get trap resolved metadata and built a new namespace/ctor/enum object on every access (`Windows.Foundation !== Windows.Foundation`). A namespace is now a plain object whose prototype is the resolving Proxy. A resolved name is defined on the object as a read-only, non-enumerable own property, so later reads never reach the Proxy. - Node: Windows.Foundation 4460 -> 12 ns, Windows.UI.Xaml.Controls.Button 11613 -> 14 ns - QuickJS: Windows.Foundation 4300 -> 80 ns, Button 12070 -> 420 ns
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On the napi engines (Node, QuickJS, Hermes, JSC), namespace objects were Proxies whose
gettrap resolved metadata and built a new namespace, constructor or enum object on every access. Each level cost several microseconds, andWindows.Foundation === Windows.Foundationwasfalse.Now a namespace is a plain object whose prototype is the resolving Proxy. The first access to a name resolves it and defines it on the object as a read-only, non-enumerable, non-configurable own property (
napi_define_properties). Later reads find the own property and never reach the Proxy, so the engine can cache them like any other property.__typeName__is an own property from the start.This is the napi counterpart of the classic change in #26.
Performance
Windows.FoundationWindows.Foundation.UriWindows.UI.Xaml.Controls.ButtonWindows.UI.Xaml.Visibility.CollapsedFor reference, the same four-property chain on a plain JS object costs about 110–210 ns on QuickJS, so the remaining QuickJS cost is the interpreter. Node RSS after the benchmark dropped from 300 MB to 50 MB.
Behaviour changes
Windows.Data.Json === Windows.Data.Json).Object.keys(namespace)stays empty.Testing
windows-napi:npm testpasses 406/406, plus new identity/read-only checks inns-proxy-test.js(42/42).packages/test/interop-check.jspasses 22/22. Identity, own read-only properties, enums and construction checked in a script.Note:
ColorHelper.ToDisplayNamecrashInvestigated and not a runtime bug. Small structs passed by value marshal correctly on both engines:
new Windows.UI.Input.Inking.InkPoint({X:1.5,Y:2.5}, 0.5).Positionround-trips exactly. The crash is a null dereference insideWindows.UI.Xaml.dll.ColorHelperships in the XAML metadata, and the C# SDK doesn't even project it outside XAML/UWP apps, so it needs a running XAML app.